It is a bit weird to render an error notice inside a dropdown. Can we render the error using the snackbar notices instead?

@mtias I thought about that as well, but in #19156 @karmatosed advised against that route:

I would advise against snackbars for this as it's not right by where the error occurs and it's also the wrong use of that style of the component.

In this light I agree with the same, the error would be to offsite from where the action happened. Nevertheless this option implemented here also feels "too contextual", which means cramped.

Tammie also provided another option where the error would be stuck to the component that implements / uses MediaReplaceFlow but that would require updates to all these components, I believe.

Correct, the drop-down wasn't only space I suggested, there was within placeholder which could also work. For me, I felt the snack bar was unexpected as not used in other cases for that.

Inline notices for async operations are tricky because the user might have scrolled or moved their attention elsewhere. The snackbar fits the paradigm better in my mind as it can also increase consistency if more blocks use it.

Agreed that the snackbar would not be ideal for the reasons stated. I would look at Tammie's proposal, having the error appear in the placeholder sounds right to me.

There's no placeholder there.

Rendering the error inside the dropdown is not a very scalable way. Rendering it on the block itself, while "geographically in context", is also not necessarily ideal as it would not work for a really really small image.

To me, the two existing notice mechanisms are appropriate to use, either the snackbar which reads the error aloud to screen readers, or the Notice, which shows at the top. Which to use depends on how serious we think the error is.

By using either of those not only do we use the same notices we use for virtually everything else, we ensure that they are visible both when the image is REALLY small, or even if you upload, then scroll the block out of view (both are fixed position).

@jasmussen which notices show "at the top"?

The <Notice /> (I believe) https://developer.wordpress.org/block-editor/components/notice/

At the moment we are using different places for messages and we unless mistaken (which might be) don't use snack bars for error messages. I think aside from this issue looking at an audit and then clearly noting in docs what area we use for what, under what situation, notices is key going forward and will help everyone. This makes it easier for people to just pop in whatever context notice applies in future.

I made this to start that process (low priority but good to do in the new year): #19277

I'll add that snackbars are great for quickly reading through an update message or something similar. An error message always takes me more time to read and understand what's going wrong. Snackbars as error messages can cause an increase in anxiety trying to read the error before it goes away... If we're going with existing notifications, I think using an error notification in the top works well.

Using default notices seems better than contextual for this kind of message, as it is generated by the, what looks like, general UI of the editor.

Media errors are most likely to happen because of system level settings or issues, therefore they're not contextual to the component that uses and displays media.

Snackbars, as @mapk also explained above, are not yet suitable for errors which need to sometimes be debugged or forwarded to other people. Contextual errors are not suited for UI elements.

Works! I added the a11y feedback label, to get a another review..

Some ideas:

• Add an ID to the notice, so the old notice will be replaced, if we get the notice again.
• Remove the notice, if the URL was updated successful.

Thanks @Soean great suggestions added them both in 5865361 !

Testing with Safari and VoiceOver, the speak message is not announced.

I guess this is because timing:

1. the notice appears and the speak message runs
2. focus is moved back to the "Upload" menu item

An aria-live assertive message can only interrupts anything the screen reader is announcing in that moment. Focus is set back to the menu item immediately after the speak message happens so the screen reader just announces the newly focused element.

I think the notice should be slightly delayed or "put in the queue" after the focus thing.

I tested adding a quick'n dirty setTimeout and that made the speak message work. I guess there's a cleaner way in Gutenberg to delay / defer a function call.

Hey @draganescu!

I tried testing this PR but I'm getting an unexpected error. I just wanted to confirm if I should be doing anything on my side in addition to just switching to this branch?

When uploading a ~18MB JPEG, I get this error:

Size Change: +293 B (0%)

Total Size: 856 kB

_{compressed-size-action}

So I added a debounced call to speak, it does work, but it seems like a hack rather than a solution. I'll see if I can get any help on a better solution.

+23 kB

That's a big change for such a small diff. I wonder if there's a bug with the bundle size bot somehow. As far as I know the deps here are externals so, they shouldn't impact the bundle size that much. Any ideas @gziolo @nerrad @jsnajdr

@draganescu sorry for squatting your PR for an unrelated thing :)

@draganescu how fresh is this branch compared to master, asking cause I'm wondering if it's because it's not rebased recently enough which could cause the big diff.

Ok, now the numbers make more sense (after a rebase). I'll open an issue on the Github action tracker to see if we can have better numbers without requiring rebases.

I just tested it and got the same results as @afercia.

So I added a debounced call to speak, it does work, but it seems like a hack rather than a solution.

If it's possible, I think the idea of putting it in the queue might work better.

@enriquesanchez but I did add that, the current version of the PR speaks the message with a delay. It seems five seconds were enough to wait for VO to speak all the stuff before the error, but if it still didnt work for you, then it means this is not the way to do it :)

Notice has a built-in speak mechanism (recently updated as of #15745), which I think has relevance here in one of two ways:

• Ideally we rely on the built-in speak behavior of notice, rather than calling speak directly. Per the previous conversation, would it be enough to change the order of the programmatic focus vs. createNotice to ensure that focus is shifted before the notice is created?
  • If both need to be announced, and can't interoperate (queue) themselves, then could we instead defer the notice creation after the delay, rather than speak? It's a bit uncomfortable to me that in addition to the code duplication of recreating the speak behavior, we also have different timing for the notice and the message.
• At the very least, we should disable the Notice's default speak handling, via speak: false option (reference), if it's going to be handled via a separate implementation.

hey @draganescu 👋

but I did add that, the current version of the PR speaks the message with a delay

Oh, I'm sorry. I must have not properly build after switching branches. I tested it again, and I heard the error being announced this time 🎉

Because focus moves back to the first menu item in the replace popover, VoiceOver announces this first. Then the error is announced. This confused me a bit.

Ideally the error could be announced before focus moves, but I'm not sure if this is technically possible?

@afercia would a scenario like the described above be correct? Should the error be announced before any focus change?

Following @aduth 's suggestion above instead of delaying the speak, I delayed the notice creation and use the built in speak mechanism instead. In my testing it worked very well. @enriquesanchez if you can please conform this is the case for you as well that'd be great!

hey @draganescu!

I just tested it and it worked as expected, so much better! 👍👍